Probe container's IP and forwarded IP for urcap endpoint by urfeex · Pull Request #614 · UniversalRobots/Universal_Robots_Client_Library

urfeex · 2026-11-03T19:45:04Z

In some situations, we might want to probe for the container's IP, while in other setups we might want to probe the forwarded http port on the host. Before, the host forward was used as soon as it was specified. With this change both are probed and the one that answers with a 200 status code is picked for installing the urcap.

codecov · 2026-11-03T19:47:02Z

Codecov Report

❌ Patch coverage is 76.66667% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.44%. Comparing base (d14edd7) to head (e144383).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
scripts/start_ursim.sh	76.66%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #614      +/-   ##
==========================================
- Coverage   83.86%   83.44%   -0.43%     
==========================================
  Files         127      127              
  Lines        8388     8408      +20     
  Branches     3829     3829              
==========================================
- Hits         7035     7016      -19     
- Misses        946      980      +34     
- Partials      407      412       +5

Flag	Coverage Δ
python_scripts	`49.69% <ø> (ø)`
start_ursim	`91.82% <76.66%> (-1.22%)`	⬇️
unit	`61.60% <ø> (+0.05%)`	⬆️
ur20-latest	`81.06% <ø> (+0.03%)`	⬆️
ur5-3.14.3	`80.80% <ø> (+0.02%)`	⬆️
ur5e-10.11.0	`?`
ur5e-10.12.0	`77.60% <ø> (-0.04%)`	⬇️
ur5e-10.7.0	`76.10% <ø> (-0.02%)`	⬇️
ur5e-5.9.4	`81.08% <ø> (+0.03%)`	⬆️
ur7e-10.13.0	`78.15% <ø> (-0.04%)`	⬇️
ur7e-10.14.0	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

Copilot

🟡 Changes recommended

On UrService timeout, startup exits without stopping the launched container, leaving URSim holding its name and ports and blocking retries even in foreground mode.

1 open issue

Stop the container when foreground startup times out · New

What changed in this PR

This PR makes URCap installation work across container-network and host-forwarded setups by selecting an UrService endpoint that responds with HTTP 200.

Changes:

Probe the container IP first, then the forwarded HTTP endpoint.
Add a configurable readiness timeout and propagate setup failure.
Update tests for container-IP preference, forwarded-endpoint fallback, and timeout behavior.

File	Description
tests/test_start_ursim.bats	Tests endpoint selection and UrService timeout handling.
scripts/start_ursim.sh	Probes available endpoints, bounds readiness waiting, and exits on setup failure.

_{Review effort: Balanced}

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

🔵 Needs a closer look

The readiness loop can exceed the configured deadline and install after it expires, while the endpoint test does not specifically verify that the installation POST uses the forwarded URL.

No open issues 🎉

2 previously missed issues

In code that hasn't changed since last review

Enforce the deadline for each probe and skip expired retries

scripts/start_ursim.sh:439

The deadline is checked only before a full round of probes, so URSIM_URSERVICE_TIMEOUT=1 can still wait roughly eleven seconds if both requests reach their five-second limit. A successful forwarded probe can also trigger installation after the deadline has expired. Check the remaining budget before each probe, cap its timeout to that budget, and skip the retry sleep once the deadline expires.

Assert the installation POST uses the forwarded URL

tests/test_start_ursim.bats:1039

This assertion also matches the forwarded readiness probe, so the test still passes if the installation POST uses the unreachable container IP or is omitted entirely. Assert the forwarded URL on the POST entry specifically, so this test verifies that the selected endpoint is actually used to install the URCap.

1 resolved since last review

Stop the container when foreground startup times out

_{Review effort: Balanced}

cursor · 2026-11-03T20:08:15Z

PR Summary

Low Risk
Changes are limited to the URSim startup script and its tests, improving Docker/CI connectivity without touching robot control or auth paths.

Overview
PolyScope X startup no longer assumes a published host port is the right way to reach UrService. post_setup_polyscopex builds a list of candidate bases (container IP first, plus the forwarded docker port endpoint when present), polls the UrService urcaps API on each until one returns 200, and uses that URL for the URCapX POST. Waiting is bounded by URSIM_URSERVICE_TIMEOUT (default 300s); on timeout the script prints what was tried and returns failure.

main now treats post-setup failure as fatal: in non-detached mode it stops the URSim container before exiting. Bats coverage adds cases for preferring the container IP, falling back to the forwarded endpoint, and timing out.

^{Reviewed by Cursor Bugbot for commit e144383. Bugbot is set up for automated code reviews on this repo. Configure here.}

cursor

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{Reviewed by Cursor Bugbot for commit e144383. Configure here.}

cursor · 2026-11-03T20:18:01Z

-    http_host="$IP_ADDRESS"
+  # UrService is reachable on the container IP from another container on ursim_net
+  # (CI). A published host port is required on Docker Desktop, where that IP is
+  # not routed. Probe both, and install the URCap through whichever answers.


PR title needs grammar fixes

Low Severity

The PR title is missing articles and uses urcap instead of URCap. A clearer wording is Probe the container IP and forwarded IP for the URCap endpoint.

^{Triggered by team rule: PR title}

^{Reviewed by Cursor Bugbot for commit e144383. Configure here.}

urfeex · 2026-11-04T09:51:23Z

As discussed offline, we close this PR as the use case might be too obscure vs. the new (mainly unused) code path added to maintain.

urfeex requested a balanced review from Copilot November 3, 2026 19:45

Copilot started reviewing on behalf of urfeex November 3, 2026 19:45

View session

Copilot AI reviewed Nov 3, 2026

View reviewed changes

Comment thread scripts/start_ursim.sh Resolved

Fix conditional check and cleanup for Docker container

e144383

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

urfeex requested a balanced review from Copilot November 3, 2026 19:53

Copilot started reviewing on behalf of urfeex November 3, 2026 19:53

View session

Copilot AI reviewed Nov 3, 2026

View reviewed changes

urfeex changed the title ~~Probe container's IP and forwarded IP for urcaps endpoint~~ Probe container's IP and forwarded IP for urcap endpoint Nov 3, 2026

urfeex marked this pull request as ready for review November 3, 2026 20:07

urfeex requested a review from a team November 3, 2026 20:07

urfeex added the enhancement New feature or request label Nov 3, 2026

cursor Bot reviewed Nov 3, 2026

View reviewed changes

urfeex closed this Nov 4, 2026

Navigation Menu

Repository navigation

Probe container's IP and forwarded IP for urcap endpoint - #614

Probe container's IP and forwarded IP for urcap endpoint#614
urfeex wants to merge 2 commits into
UniversalRobots:masterfrom
urfeex:start_ursim_probe_multiple

urfeex commented Nov 3, 2026

Uh oh!

codecov Bot commented Nov 3, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

cursor Bot commented Nov 3, 2026 •

edited

Loading

Uh oh!

cursor Bot left a comment

Uh oh!

cursor Bot Nov 3, 2026

Uh oh!

urfeex commented Nov 4, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

urfeex commented Nov 3, 2026

Uh oh!

codecov Bot commented Nov 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

cursor Bot commented Nov 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot Nov 3, 2026

Choose a reason for hiding this comment

PR title needs grammar fixes

Uh oh!

urfeex commented Nov 4, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented Nov 3, 2026 •

edited

Loading

cursor Bot commented Nov 3, 2026 •

edited

Loading